-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Remove JSAPI databar code #1758
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1758 +/- ##
==========================================
+ Coverage 45.99% 46.01% +0.01%
==========================================
Files 626 626
Lines 37576 37559 -17
Branches 9450 9443 -7
==========================================
- Hits 17284 17281 -3
+ Misses 20237 20223 -14
Partials 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
We've discussed the pyhon api being on ul.table... the JS would always be needed regardless of where the api originates from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just rip the API part out of databars, not the whole UI part (though it will be unused until we do wire it up to API).
Building into deephaven.ui we'll need to extend the model with our data bar options (so where UITable.tsx
calls IrisGridModelFactory.makeModel
, we'll need some way to add options in there or extend the model returned or have our own factory function or something).
Side note, in the future having plugins for cell renderers might be neat (customer I met with today was asking about them) but not our top priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only changes we need is for dataBarOptionsForCell
to throw, and remove formatDataBar
from the Format
type in dh.types.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dataBarOptionsForCell
throws from IrisGridModel
, so just removing it from the table model template. renderTypeForCell
returns text
from GridModel
, so removing that from the table model template as well
8c372dc
to
7034706
Compare
Removing databar specific JSAPI code since this API will be moved to dh.ui at some point